Security : FeeDistributor centralization risks + grief surface#10
Security : FeeDistributor centralization risks + grief surface#10philpof102-svg wants to merge 2 commits into
Conversation
|
Upgraded with merge-ready fix ( Two of the three issues fixed in this PR ; the third (sink revert grief) is deferred to a follow-up to keep this diff reviewable. 1) Keeper-share floor uint256 public constant MIN_KEEPER_SHARE_BPS = 25; // 0.25 %
function setSplit(uint256 _n, uint256 _u, uint256 _k) external {
...
if (_k < MIN_KEEPER_SHARE_BPS) revert KeeperShareTooLow();
...
}Stops the multi-update walk-to-zero where owner shifts keeperShareBps by MAX_BPS_CHANGE per call until permissionless cron stops being profitable. 2) Sink timelock (replace uint256 public constant SINK_TIMELOCK = 2 days;
address public pendingNodeStaking;
address public pendingUserStaking;
uint256 public pendingSinksEta;
function queueSinks(address _n, address _u) external { ... } // emits SinksQueued
function executeSinks() external { ... } // only after eta
function cancelSinks() external { ... }Stakers get a 2-day window to exit if the queued sink is malicious. Monitor Migration : scripts/UI that called The sink-revert grief (where a malicious sink reverts on |
…itlawb#10) Three centralization risks in GitlawbFeeDistributor admin path : 1. **Keeper rug** : owner could drop keeperShareBps to 0 over multiple updates (5% per step, 4 weeks to zero), stopping the permissionless cron and forcing distributions through owner-controlled paths. Fix: MIN_KEEPER_SHARE_BPS = 25 (0.25 %) hard floor in setSplit(). 2. **Sink swap with no notice** : setSinks() was instantaneous. Owner could swap nodeStaking/userStaking to a malicious contract that silently captures the next distribute() payout, with stakers having no exit window. Fix: replaced setSinks with queue/execute/cancel pattern gated by SINK_TIMELOCK = 2 days. Owner queues a swap; it executes only after the timelock. Stakers can monitor SinksQueued events and exit during the 2-day window. 3. (Sink revert grief is addressed by callers wrapping in try/catch in a follow-up — out of scope for this minimal fix to keep the diff reviewable. Tracked separately.) New surface : - constant: MIN_KEEPER_SHARE_BPS, SINK_TIMELOCK - storage: pendingNodeStaking, pendingUserStaking, pendingSinksEta - events: SinksQueued, SinksCancelled - errors: KeeperShareTooLow, NoPendingSinks, TimelockNotElapsed - removed: setSinks(address, address) - added: queueSinks, executeSinks, cancelSinks Migration : front-end / scripts that called setSinks must split into queueSinks → wait 2 days → executeSinks. Related to PR Gitlawb#10 bug report.
1 participant
Three combined FeeDistributor findings :
Full PoC + 7-day timelock fix pattern in BUG_REPORT_feedist_centralization_and_griefing.md.
6th PR in continuous Gitlawb audit (#5 #6 #7 #8 #9 #10). Reporter @philpof102-svg.